Skip to content

Conversation

@aabrown100-git
Copy link
Collaborator

@aabrown100-git aabrown100-git commented Sep 23, 2025

Current situation

Resolves #291

Release Notes

  • New BoundaryCondition class to handle uniform and spatially varying BC values from a .vtp filepath provided in the input file.
  • New RobinBoundaryCondition class, derived from BoundaryCondition, replaces all existing code for handling Robin BCs and adds capability for defining spatially varying Robin BCs.
  • struct and ustruct test cases for spatially varying Robin BC.
  • Other additions to CmMod.h/cpp and VtkData.h/cpp necessary to implement the BoundaryCondition class.

Documentation

Should add to svMultiPhysics documentation.

Testing

  • Added struct and ustruct tests for spatially varying Robin BCs.

Code of Conduct & Contributing Guidelines

aabrown100-git and others added 22 commits September 18, 2025 13:37
…ls with multiple processors. Other improvements to make
…different from struct result, so could not use result_002.vtu from the struct case
…ced custom exception classes for VTP file errors and node count mismatches. Updated point matching logic to use squared distance for efficiency. Removed redundant map clearing operations during initialization. Also, put test cases number of timesteps back to 2
…tion. Add generic flag handling in BoundaryCondition
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR implements variable Robin boundary conditions and introduces a generic boundary condition class to handle spatially variable boundary condition values from VTP files. The implementation adds capability for defining Robin boundary conditions with spatially varying stiffness and damping properties.

  • Introduces generic BoundaryCondition class for reading spatially variable arrays from VTP files
  • Implements RobinBoundaryCondition class that replaces existing uniform Robin BC functionality
  • Adds test cases demonstrating spatially variable Robin boundary conditions for both struct and ustruct physics

Reviewed Changes

Copilot reviewed 55 out of 55 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
BoundaryCondition.h/cpp New generic BC class for handling VTP-based spatially variable arrays
RobinBoundaryCondition.h/cpp Robin-specific BC class extending BoundaryCondition
set_bc.h/cpp Updates to use new RobinBoundaryCondition class in Robin BC assembly
VtkData.h/cpp Added const correctness and copy constructors for VTP data handling
ComMod.h Integration of RobinBoundaryCondition into bcType structure
CmMod.h/cpp Added MPI communication methods for BC distribution
test files New test cases for spatially variable Robin BC functionality
Comments suppressed due to low confidence (1)

Code/Source/solver/BoundaryCondition.cpp:1

  • Using eval() is a security risk even with a restricted namespace. Consider implementing a proper expression parser or mathematical expression library instead of eval() for safer evaluation of user-provided mathematical expressions.
/* Copyright (c) Stanford University, The Regents of the University of California, and others.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@aabrown100-git aabrown100-git changed the title Variable Robin boundary condition and generic boundary class Variable Robin boundary condition and generic boundary condition class Sep 23, 2025
@codecov
Copy link

codecov bot commented Sep 23, 2025

Codecov Report

❌ Patch coverage is 64.97696% with 152 lines in your changes missing coverage. Please review.
✅ Project coverage is 67.12%. Comparing base (949d1a6) to head (ee43996).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
Code/Source/solver/BoundaryCondition.cpp 84.29% 41 Missing ⚠️
Code/Source/solver/VtkData.cpp 12.82% 34 Missing ⚠️
Code/Source/solver/BoundaryCondition.h 13.15% 33 Missing ⚠️
Code/Source/solver/CmMod.cpp 25.00% 33 Missing ⚠️
Code/Source/solver/set_bc.cpp 73.91% 6 Missing ⚠️
Code/Source/solver/RobinBoundaryCondition.h 71.42% 4 Missing ⚠️
Code/Source/solver/baf_ini.cpp 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #437      +/-   ##
==========================================
+ Coverage   67.07%   67.12%   +0.04%     
==========================================
  Files         166      169       +3     
  Lines       33748    34151     +403     
  Branches     5674     5726      +52     
==========================================
+ Hits        22638    22923     +285     
- Misses      10972    11090     +118     
  Partials      138      138              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ktbolt
Copy link
Collaborator

ktbolt commented Sep 23, 2025

@aabrown100-git Great work ! And nice to clean up VtkData and add more CmMod MPI interface functions.

@ktbolt
Copy link
Collaborator

ktbolt commented Sep 25, 2025

@aabrown100-git There is a lot going on in the BoundaryCondition constructors, like reading files, that can throw. I think it is a good practice to catch exceptions within constructors ?

@aabrown100-git
Copy link
Collaborator Author

@ktbolt Sounds good about catching exceptions within the constructors. I can put the constructor code within a try-catch block. But in the catch part, what should I do with the exceptions? Just throw it again?

…tilities/generate_boundary_condition_data/ directory, and update READMEs to reflect new location
@aabrown100-git
Copy link
Collaborator Author

Integration tests are currently failing to due errors when trying to build svZeroDSolver. I made an issue in svZeroDSolver and proposed a solution to try.

@ktbolt
Copy link
Collaborator

ktbolt commented Sep 25, 2025

@aabrown100-git We will need to throw it again to return from the constructor.

Some software systems need to recover from an exception and continue running but in our case the solver needs to shut down when it can't read in data it needs.

@aabrown100-git
Copy link
Collaborator Author

@ktbolt Got it, so just what I added in this commit?

@aabrown100-git
Copy link
Collaborator Author

@ktbolt Do you think okay to merge now, or would you like to see additional changes?

@ktbolt
Copy link
Collaborator

ktbolt commented Sep 26, 2025

@aabrown100-git Have you tested this on a large model, maybe 1M elements ? Just curious.

@aabrown100-git
Copy link
Collaborator Author

@ktbolt Not yet, but I will do that soon with my cardiac mechanics model.

Copy link
Collaborator

@ktbolt ktbolt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let it be merged !

@ktbolt ktbolt merged commit aac9182 into SimVascular:main Sep 26, 2025
8 checks passed
@aabrown100-git aabrown100-git deleted the variable_Robin_BC branch September 26, 2025 01:43
kko27 pushed a commit to kko27/svMultiPhysics that referenced this pull request Dec 17, 2025
SimVascular#437)

* Initial implementation and test case. Works with 1 processor, but fails with multiple processors. Other improvements to make

* Put debugging statements inside debugging blocks

* Add spatially_variable_robin test to pytest suite

* Rename to VariableRobinBCData, initial VariableRobinBCData once, instead of at every Newton iteration

* Add copyright statement

* Renamed to RobinBC, partial implementation of parallelization

* Refactored distribute method to use gatherv and scatterv. Dealing with some compilation issues now.

* Passes test in parallel, fails with index error with 1 proc

* Fixed bug, now passes test in serial

* clean up debugging statements to make code more readable

* Refactor vtp file reading to accept vector of array names, more generic this way

* Split implementation into a BC base class and a RobinBC derived class

* Update README.md and add ustruct test case. ustruct result is fairly different from struct result, so could not use result_002.vtu from the struct case

* Refactor BC class to improve error handling and code clarity. Introduced custom exception classes for VTP file errors and node count mismatches. Updated point matching logic to use squared distance for efficiency. Removed redundant map clearing operations during initialization. Also, put test cases number of timesteps back to 2

* Rename BC and RobinBC classes as BoundaryCondition and RobinBoundaryCondition classes.

* Clean up constructors

* Remove extraneous comments and debugging statements

* Reformat code

* Move flag to apply in normal direction only inside RobinBoundaryCondition. Add generic flag handling in BoundaryCondition

* Split up distribute method into smaller functions

* Add copyrights

* Implement copy-and-swap idiom, add noexcept to appropriate methods

* Catch exceptions in constructors

* Move generate_load.py and generate_spatially_variable_robin.py into utilities/generate_boundary_condition_data/ directory, and update READMEs to reflect new location

* Initialize members with default values and use default default constructor

* Remove defined_ flag, just check if arrays are allocated, add new exception classes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Spatially-varying boundary condition values

2 participants